Lesson Plan JS -> TS UI conversion and implementation enhancements - #8559

Fixes the client-side cause of a production incident where two students could no longer finalise any
submission in their course, and converts the lesson plan bundle it lives in to TypeScript along the way.

The lesson plan editor sent a PATCH on every datetime change, with no debouncing and no waiting for a
prior request to finish. Two of those requests overlapping enqueued two concurrent
CoursewidePersonalizedTimelineUpdateJob runs, which raced each other into creating duplicate
Course::PersonalTime rows — after which every personalisation run for those students raised
Validation failed: Course user has already been taken and rolled back their submission.

Typing a date passes through valid intermediate values — keying 1 then 5 for 15:00 yields 01:00
first — and each valid value fired onChange. The only guard, sameDate(oldDate, newDate) in
DateCell, compared against props.fieldValue, which is the Redux value and only catches up after
the request resolves. So a second event inside the ~0.3s round trip compared against a stale value,
passed, and dispatched.

The client no longer emits concurrent writes for the same lesson plan item, so this trigger
is closed. Further mitigation work on the backend side of the incident is planned.

Key changes

1. Debounced, serialised saves (ItemRow, MilestoneRow)

Modelled on the submission answers autosave loop, with one deliberate difference.

• 1500ms debounce (FIELD_LONG_DEBOUNCE_DELAY_MS), so a typed date collapses into one request.
• Merged pending payload — editing start, bonus and end on one row produces a single PATCH rather
  than three. This matters more than the debounce alone: the request axis is per item, so merging is
  what keeps it to one job per edit.
• At most one request in flight per row. Edits arriving mid-flight stay pending and flush on
  completion.
• The diff happens at flush, against the saved state, not when the edit is queued. An in-flight
  request may have moved the saved state on by then; after a failure it will not have.
• A LoadingIndicator beside the row title from first keystroke until the save resolves, and the
  published toggle is disabled while saving.

The answers loop uses a clientVersion logical clock to let overlapping saves reconcile afterwards.
That is right for a text editor and wrong here — we want no overlap. Serialising per row gives the
stronger guarantee with less state, and avoids a latent bug the version approach would have needed to
handle: itemUpdated writes client-supplied values into the store, so two responses arriving out of
order would leave the grid showing a value the server does not have.

2. Edits the editor holds back

Two kinds of edit are never sent, for different reasons:

• An empty start_at is unfinished input rather than an edit — the user is part-way through
  retyping a date. Nothing is queued and nothing is said, because nothing has gone wrong yet.
• An end date before the start date is a finished edit that the server would reject
  (Course::ReferenceTime#start_at_cannot_be_after_end_at). DateCell checks the start/end pair as
  the edit would leave it and, if it is out of order, shows the error on the field and returns before
  queueing — so no request goes out and no debounce is running until the user corrects it. Editing
  start_at cannot trigger this, because the end dates shift with it.

The message is formTranslations.startEndDateValidationError, which the event form's yup schema
already uses for the same rule, so the inline grid and the dialog report identically and there is no
new string to translate.

bonus_end_at is deliberately not guarded: the server validates only start against end, so blocking
it client-side would refuse edits the server would accept.

3. Failure handling

The rows own their own outcome messages now; updateItem and updateMilestone report a boolean and
say nothing. That is what makes the next two behaviours possible:

• A failed save puts the saved values back on screen. The store never took the rejected value, so
  nothing else would move the field back — DateTimePicker holds its own display state, hence the
  remount key.
• A superseded failure stays silent. If a newer edit was queued while the failed request was in
  flight, the user gets one verdict, from the last edit, rather than a failure followed by a success.

One condition — is anything pending at resolution? — drives both.

4. Form handlers out of Redux (EventFormDialog, MilestoneFormDialog)

showEventForm/showMilestoneForm stashed an onSubmit function in the store — not serialisable,
silently tolerated in production, noisy in development, and a blocker for RTK's serializableCheck.

Both dialogs are now controlled by whoever opens them, taking open, onClose, formTitle,
initialValues and onSubmit as props, with local submitting state. Each opener renders its own
dialog; the global mounts in LessonPlanLayout are gone. This follows CreateRenameTimelinePrompt in
the reference-timelines bundle.

The operations previously closed the dialog themselves via dispatch(actions.hideMilestoneForm())
inside .then(). Their .catch() swallows errors, so the returned promise resolved either way and the
caller could not tell success from failure. The four create/update operations are now
Operation<boolean>, and the dialog closes itself only on success — so a failed submit still stays
open with its field errors, exactly as before.

eventForm.js and milestoneForm.js are deleted, along with their combineReducers entries, four
action creators and four action types. Nothing read them once the handlers moved.

5. TypeScript and createSlice

27 files converted. All the touched class components became function components using
useAppDispatch/useAppSelector/useTranslation, so connect, injectIntl, PropTypes and
FormattedMessage are gone from them — the last of those being the deprecation we are phasing out.

Both remaining reducers are now createSlice, matching submission/reducers/history and
submission/reducers/scribing. flagsSlice picks up the flags that arrive with the lesson plan fetch
through extraReducers on lessonPlanActions.loadSucceeded rather than sharing a string constant.
actionTypes is gone entirely, and constants.ts with it: the edit-page columns are now a
LESSON_PLAN_EDIT_COLUMNS const array and a LessonPlanEditColumn union, so
editPageColumnsVisible is a Record<LessonPlanEditColumn, boolean> and the column lookups are
checked properties rather than string indexing.

With the reducers typed, combineReducers infers the slice and every compatibility cast in the
components disappeared — state.lessonPlan.lessonPlan.groups and friends now type-check directly.

6. Rendering cost

Converting connect to hooks silently drops two things it provided for free, both fixed here:

• Shallow comparison of mapped props. LessonPlanLayout briefly selected the whole
  lessonPlan slice, whose identity changes on any change within it — so every filter toggle and every
  item save re-rendered <Outlet /> and therefore the entire routed page. Selectors are now field by
  field, and ItemRow selects visibilityByType[type] as a boolean so only the rows whose own type
  changed re-render.
• React.memo. connect() wraps its component; a plain function component gets nothing. ItemRow
  and MilestoneRow are memoised again — each row renders up to three MUI date pickers, so an
  unnecessary parent render is expensive.

Both dropdowns also mark their dispatch as a startTransition, following SearchField, so showing a
hidden type does not block the click while its rows mount.

Behaviour changes worth reviewing

These are real changes, not pure refactors:

• A failed save reverts the field to the saved value. Previously the rejected value stayed on
  screen with only a toast to say it had not saved. This is the most visible change in the PR.
• An empty start_at no longer sends a request. It is required server-side, so an empty field is a
  transient state on the way to a new date rather than an update the server would accept. Previously it
  PATCHed and 4xx'd. Applies to both items and milestones.
• An end date before the start date is reported on the field instead of being sent and bounced.
  Previously it PATCHed, the server rejected it, and the user got a toast; now the field carries the
  same message the event form shows, and the edit waits there until corrected.
• createEvent/updateEvent now receive setError. Their signatures always declared it, but the
  JS callers passed one argument short and silently dropped it — TypeScript caught this on conversion.
  Server-side validation errors now reach the event form's fields, as they already did for milestones.
• Clearing start_at no longer wipes the end dates. The old DateCell computed the start-shift
  unconditionally, so a cleared start produced moment(null).diff(...) → NaN → null for end_at
  and bonus_end_at too.
• AdminTools guards lesson_plan_item_type?.[0]. Moving initialValues from dispatch-time into
  render meant it evaluated on every render; any event item lacking that field would previously have
  been fine until you clicked edit.
• LessonPlanLayout dropped a children prop that was declared required in propTypes but never
  passed or used — the component renders <Outlet />. Its stale @ts-ignore in the router went too,
  now that the component no longer uses connect.

Testing

tsc --noEmit, eslint and prettier are clean. 33 tests pass across 11 suites in the bundle, up from
24 across 9; the full client suite is green.

Each regression test was verified to fail without its fix rather than pass vacuously:

• sends one request when a date is edited several times in quick succession — two rapid changes,
  asserts exactly one PATCH carrying the final value. This is the incident, in a test.
• retries the same value after a failed save — fails the PATCH, asserts the field reverts, re-enters
  the rejected value and asserts a second request goes out.
• sends nothing while start date is empty, and sending resumes once it is valid.
• holds an end date that precedes the start date, and reports it — asserts the message appears and
  that no request goes out even after the debounce window elapses; a sibling test asserts the edit is
  sent once the ordering is corrected and the error clears.
• Both visibility dropdowns get a test that opens the menu, clicks an entry and asserts the tick
  flips — a round trip through the reducer, which is what a reducer-level test would have missed.

The four dialog tests got simpler: they used to render the opener and the dialog side by side and let
Redux connect them, which was the coupling itself.

Proving the absence of a request needs the debounce window to actually elapse, so two tests wait it
out via a settleDebounce helper. That adds roughly 4s to the suite. Fake timers would avoid it but
interact badly with RTL's waitFor and the MUI pickers.

Notes / follow-ups

• updateOrAppend merges rather than replaces, and the slice preserves that. An item update
  dispatches only the changed fields, so a straight replace would blank title and type on every save.
• Seven as unknown as Partial<AppState> remain, all in tests. Unrelated to the reducers:
  Partial<AppState> only permits omitting whole slices, and these tests seed a handful of fields.
• A pending edit is still lost on unmount — useDebounce cancels rather than flushes, so navigating
  away within 1500ms drops it. Inherited from the shared hook; the answers loop behaves the same way.
• 10 .jsx files remain in the bundle — the two form bodies, LessonPlanNav, and the
  LessonPlanShow subtree.
• Showing a hidden item type mounts its rows' date pickers, each constructing its own
  LocalizationProvider. Hoisting that provider out of DateTimePicker would cut it to one per page.
  Despite living in lib/, that component has only two consumers, both in this bundle — so the change
  is smaller than it looks, but it is still its own change.